feat: enhance Binder with cross-field validation #23360
Conversation
Adds Binder.Binding.value() to be used in field binding validators to read value of another field. The Binder automatically runs validators inside a ComponentEffect#effect context, making validators and converters reactive to signal changes. Part of #23298
tltv
force-pushed
the
feat/binder-signal-validation
branch
from
a0fa6ef to
2569dd3
Compare
Bean level validators throws Binder.InvalidSignalUsageError error when Signal.value() is called in them. Fixes: #23364
flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Outdated
Show resolved
Hide resolved
| if (signalRegistration == null | ||
| && getField() instanceof Component component) { | ||
| initialSignalRegistration = true; | ||
| signalRegistration = ComponentEffect.effect(component, () -> { |
There was a problem hiding this comment.
As I understand, this makes validators and converters signal-reactive for validation purposes. Given that I use a signal in my converter function, what happens regarding value updates arising from a signal-triggered change?
After reading the code, seems that such updates won't make their way to fields. If so, is it intentional? Would it make sense to run converters outside of effect context?
There was a problem hiding this comment.
It's intentionally running also converters inside the effect. But in practice I don't believe signals are actually used in converters. If they are, then validation reruns also on those signal value changes.
I didn't see earlier nice technical solution to leave converters out of scope here. But now I see one, it's to wrap each converter execution inside UsageTracker.untracked(...). Do you think this would make sense?
There was a problem hiding this comment.
I'll change this to leave converters out, if it ends up being as simple change as it seems.
There was a problem hiding this comment.
Need to update javadoc still, otherwise untracking works fine.
flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Outdated
Show resolved
Hide resolved
flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Outdated
Show resolved
Hide resolved
flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Outdated
Show resolved
Hide resolved
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
Few minor comments, but looks good to me and also works as expected with an example from the ticket.
* feat: add binder validation status signal Adds Binder.getValidationStatus() method to get read-only signal for BinderValidationStatus. Fixes: #23300 * fix single binding validation status change * fix unresolved validation status change
|
Adds
Binder.Binding.value()to be used in field binding validators to read value of another field. The Binder automatically runs validators inside aComponentEffect#effectcontext, making validators and converters reactive to signal changes.Fixes: #23298